Add HTML email format support with wizard configuration - #12
Merged
Conversation
When the visibility split is available (private_minutes / public_minutes in actions data), the forecast now projects only private usage against quota limits and labels itself "Monthly Forecast — private repos". Public minutes are shown as an informational footnote below the table since they are free. The Actions section also surfaces the private/public minute breakdown when the split is present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
email_format was a hidden config-only setting with no UI path to change it. The wizard now exposes a Select widget (plain text / HTML) in the Report options step, seeds it from config on load, saves it on next, and shows the chosen format in the review summary. GitHub Actions workflow updated to --email-format html to match local config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 6 new tests covering load_initial_data, save_options_step, and review_summary for the email_format field (WizardEmailFormatFlowTests) - Guard wizard __init__ import behind try/except so setup_wizard_flow is importable without textual installed; this also unblocks the previously-erroring test_setup_wizard_visibility.py (2 pre-existing tests now run instead of erroring at collection time) - config.example.toml: add comment explaining that config.toml is gitignored but its settings are baked into the committed workflow YAML via the setup wizard, so changes require re-rendering + committing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds public/private usage breakdowns to text and HTML reports. The setup wizard supports persisted ChangesEmail usage reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Profile
participant SetupWizardFlow
participant SetupWizardScreen
participant Workflow
Profile->>SetupWizardFlow: load email_format
SetupWizardFlow->>SetupWizardScreen: display text or html
SetupWizardScreen->>SetupWizardFlow: return selected format
SetupWizardFlow->>Profile: persist email_format
Workflow->>Profile: read email format
Workflow->>Workflow: pass --email-format to email report
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tive complexity _format_forecast_section and _format_html_forecast_section each hit complexity 17 (limit 15) due to the nested has_split → pub_min/pub_mb conditional block. Extracted _public_repos_text_note and _public_repos_html_note as module-level helpers, dropping each function back to ~13. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rror guard, validate email_format, add tests
MED-1: _format_actions_section now uses "public_minutes" as the split
signal (was "private_minutes"), matching the forecast sections and HTML
formatter so both sides of the same report use the same predicate.
MED-2: gui/wizard/__init__.py narrows except ImportError to
ModuleNotFoundError to avoid silently swallowing real import bugs in
future code that might import from setup_wizard_screen.
MED-3: load_initial_data normalizes email_format via .lower() and
validates membership in {"text","html"}, defaulting to "text". Prevents
a Textual InvalidSelectValueError crash when the config has a
non-canonical value like "HTML".
LOW-1: Added docstrings to _public_repos_text_note and
_public_repos_html_note; added 9 new tests covering the has_split paths
in both text and HTML formatters and the helper return values.
LOW-4: Simplified the Select guard in setup_wizard_screen from
"fmt and fmt is not Select.BLANK" to "fmt in ('text','html')" to make
the intent explicit.
LOW-6: Added two render_workflow round-trip tests asserting that
email_format="html" and email_format="text" in the profile config each
produce the correct --email-format flag in the rendered YAML.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
actual value must precede expected value ("") per S3415.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/github_usage/gui/wizard/__init__.py`:
- Around line 3-8: Update the ModuleNotFoundError handler around the
SetupWizardScreen import to re-raise missing internal wizard dependencies; only
suppress the exception when exc.name identifies the optional textual dependency,
including the existing behavior for an absent module name.
In `@tests/test_email_report.py`:
- Around line 835-961: Extract the public/private usage, public-repository note,
and forecast visibility tests from test_email_report.py into a focused module
such as test_email_report_visibility.py. Preserve all test names, imports,
fixtures, assertions, and behavior while keeping the original module focused and
below the project’s preferred file size.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 298d07a8-c3f3-44f3-b950-28c7bd93f4ee
📒 Files selected for processing (10)
.github-usage/config.example.toml.github/workflows/email-report.ymlsrc/github_usage/email_report_html.pysrc/github_usage/email_report_text.pysrc/github_usage/gui/wizard/__init__.pysrc/github_usage/gui/wizard/setup_wizard_flow.pysrc/github_usage/gui/wizard/setup_wizard_screen.pytests/test_email_report.pytests/test_setup_wizard_visibility.pytests/test_setup_workflow.py
Comment on lines
+3
to
+8
| try: | ||
| from .setup_wizard_screen import SetupWizardScreen | ||
|
|
||
| __all__ = ["SetupWizardScreen"] | ||
| __all__ = ["SetupWizardScreen"] | ||
| except ModuleNotFoundError: | ||
| pass |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'except ModuleNotFoundError|^from \.' \
src/github_usage/gui/wizard/__init__.py \
src/github_usage/gui/wizard/setup_wizard_screen.pyRepository: kgrizz-git/github-usage
Length of output: 1818
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- wizard package files ---\n'
git ls-files src/github_usage/gui/wizard | sort
printf '\n--- setup_wizard_screen.py outline ---\n'
ast-grep outline src/github_usage/gui/wizard/setup_wizard_screen.py || true
printf '\n--- setup_wizard_screen.py first 220 lines ---\n'
sed -n '1,220p' src/github_usage/gui/wizard/setup_wizard_screen.py
printf '\n--- textual version constraints ---\n'
for f in pyproject.toml requirements.txt requirements*.txt setup.py setup.cfg; do
if [ -f "$f" ]; then
echo "### $f"
rg -n 'textual|dependency|requires-python|dependencies|optional' "$f" || true
fi
doneRepository: kgrizz-git/github-usage
Length of output: 252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- wizard package files ---'
git ls -1 src/github_usage/gui/wizard | sort
printf '%s\n' ''
printf '%s\n' '--- setup_wizard_screen.py outline ---'
ast-grep outline src/github_usage/gui/wizard/setup_wizard_screen.py || true
printf '%s\n' ''
printf '%s\n' '--- setup_wizard_screen.py first 220 lines ---'
sed -n '1,220p' src/github_usage/gui/wizard/setup_wizard_screen.py
printf '%s\n' ''
printf '%s\n' '--- textual version constraints ---'
for f in pyproject.toml requirements.txt setup.py setup.cfg; do
if [ -f "$f" ]; then
echo "### $f"
rg -n 'textual|dependency|requires-python|dependencies|optional' "$f" || true
fi
doneRepository: kgrizz-git/github-usage
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- wizard package files ---'
find src/github_usage/gui/wizard -maxdepth 1 -type f \( -name '*.py' -o -name '*.pyi' \) | sort
printf '%s\n' ''
printf '%s\n' '--- setup_wizard_screen.py outline ---'
ast-grep outline src/github_usage/gui/wizard/setup_wizard_screen.py || true
printf '%s\n' ''
printf '%s\n' '--- setup_wizard_screen.py first 240 lines ---'
sed -n '1,240p' src/github_usage/gui/wizard/setup_wizard_screen.py
printf '%s\n' ''
printf '%s\n' '--- textual version constraints ---'
for f in pyproject.toml requirements.txt setup.py setup.cfg; do
if [ -f "$f" ]; then
echo "### $f"
rg -n 'textual|dependency|requires-python|dependencies|optional' "$f" || true
fi
doneRepository: kgrizz-git/github-usage
Length of output: 11674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- setup_wizard_flow.py first 80 lines ---'
sed -n '1,80p' src/github_usage/gui/wizard/setup_wizard_flow.py
printf '%s\n' ''
printf '%s\n' '--- relevant imports / textual dependency occurrences ---'
for f in pyproject.toml README.md docs/** src/github_usage/gui/wizard/setup_wizard_screen.py src/github_usage/gui/wizard/setup_wizard_flow.py; do
[ -f "$f" ] || continue
echo "### $f"
rg -n 'SetupWizardScreen|wizard|textual|ModuleNotFoundError|optional|extra' "$f" || true
done
python3 - <<'PY'
class FakeScreen: pass
try:
import textual.containers
textual_dependency_present = True
except ImportError:
textual_dependency_present = False
if textual_dependency_present:
import textual
root = textual.__file__ or ""
for name in ("textual.containers", "textual.screen", "text"):
try:
mod = __import__(name)
resolved = getattr(mod, "__file__", None) or None
is_textual = (resolved and resolved.lower().startswith(root[:].lower())) if root else None
print(f"{name}: present={True}, file={resolved}, under_textual={is_textual}")
except Exception as exc:
print(f"{name}: present=False, error={type(exc).__name__}: {exc}")
try:
mod = __import__("missing_internal_wizard_dep")
except ModuleNotFoundError as exc:
root = textual.__file__ or ""
print(f"internal missing dependency:")
print(f" exc.name={exc.name!r}")
print(f" startswith_textual={exc.name.startswith('textual') if exc.name else False}")
else:
print("textual not importable in this environment; cannot inspect package origin reliably.")
PYRepository: kgrizz-git/github-usage
Length of output: 18205
Do not suppress missing internal wizard modules.
This handler also catches ModuleNotFoundError raised by imports inside .setup_wizard_screen. If a local wizard dependency is missing, SetupWizardScreen is omitted silently and failures can appear later as an unknown import. Re-raise missing modules unless exc.name identifies the optional textual dependency.
try:
from .setup_wizard_screen import SetupWizardScreen
__all__ = ["SetupWizardScreen"]
except ModuleNotFoundError as exc:
if not exc.name or not exc.name.startswith("textual"):
raise
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/github_usage/gui/wizard/__init__.py` around lines 3 - 8, Update the
ModuleNotFoundError handler around the SetupWizardScreen import to re-raise
missing internal wizard dependencies; only suppress the exception when exc.name
identifies the optional textual dependency, including the existing behavior for
an absent module name.
Comment on lines
+835
to
+961
| def test_format_actions_section_shows_split_when_public_minutes_present(self): | ||
| from github_usage.email_report_text import _format_actions_section | ||
|
|
||
| data = { | ||
| "actions": { | ||
| "minutes": 1500.0, | ||
| "minutes_limit": 2000, | ||
| "minutes_percent": 75.0, | ||
| "storage_avg_mb": 200.0, | ||
| "storage_limit_mb": 500, | ||
| "storage_percent": 40.0, | ||
| "private_minutes": 1200.0, | ||
| "public_minutes": 300.0, | ||
| }, | ||
| "monthly_costs": {"actions": {"net": 0.0}}, | ||
| } | ||
| lines = _format_actions_section(data) | ||
| joined = "\n".join(lines) | ||
| self.assertIn("private: 1,200.0 min quota-counted", joined) | ||
| self.assertIn("public: 300.0 min free", joined) | ||
|
|
||
| def test_format_actions_section_no_split_without_public_minutes(self): | ||
| from github_usage.email_report_text import _format_actions_section | ||
|
|
||
| data = { | ||
| "actions": { | ||
| "minutes": 500.0, | ||
| "minutes_limit": 2000, | ||
| "minutes_percent": 25.0, | ||
| "storage_avg_mb": 100.0, | ||
| "storage_limit_mb": 500, | ||
| "storage_percent": 20.0, | ||
| }, | ||
| "monthly_costs": {"actions": {"net": 0.0}}, | ||
| } | ||
| lines = _format_actions_section(data) | ||
| joined = "\n".join(lines) | ||
| self.assertNotIn("private:", joined) | ||
| self.assertNotIn("public:", joined) | ||
|
|
||
| # --- LOW-1: new public-repos footnote helpers --- | ||
|
|
||
| def test_public_repos_text_note_returns_note_with_minutes_and_storage(self): | ||
| from github_usage.email_report_text import _public_repos_text_note | ||
|
|
||
| note = _public_repos_text_note({"public_minutes": 500.0, "public_storage_avg_mb": 12.5}) | ||
| self.assertIn("500.0 min", note) | ||
| self.assertIn("12.5 MB avg storage", note) | ||
|
|
||
| def test_public_repos_text_note_omits_storage_line_when_zero(self): | ||
| from github_usage.email_report_text import _public_repos_text_note | ||
|
|
||
| note = _public_repos_text_note({"public_minutes": 100.0, "public_storage_avg_mb": 0.0}) | ||
| self.assertIn("100.0 min", note) | ||
| self.assertNotIn("MB", note) | ||
|
|
||
| def test_public_repos_text_note_returns_empty_when_both_zero(self): | ||
| from github_usage.email_report_text import _public_repos_text_note | ||
|
|
||
| self.assertEqual( | ||
| _public_repos_text_note({"public_minutes": 0.0, "public_storage_avg_mb": 0.0}), "" | ||
| ) | ||
|
|
||
| def test_public_repos_html_note_returns_html_with_data(self): | ||
| from github_usage.email_report_html import _public_repos_html_note | ||
|
|
||
| note = _public_repos_html_note({"public_minutes": 200.0, "public_storage_avg_mb": 5.0}) | ||
| self.assertIn("200.0 min", note) | ||
| self.assertIn("visibility-tag", note) | ||
| self.assertIn("5.0 MB avg storage", note) | ||
|
|
||
| def test_public_repos_html_note_returns_empty_when_both_zero(self): | ||
| from github_usage.email_report_html import _public_repos_html_note | ||
|
|
||
| self.assertEqual( | ||
| _public_repos_html_note({"public_minutes": 0.0, "public_storage_avg_mb": 0.0}), "" | ||
| ) | ||
|
|
||
| def test_format_forecast_section_shows_private_scope_when_split_available(self): | ||
| from datetime import date | ||
|
|
||
| from github_usage.email_report_text import _format_forecast_section | ||
|
|
||
| data = { | ||
| "actions": { | ||
| "minutes": 1500.0, | ||
| "minutes_limit": 2000, | ||
| "storage_avg_mb": 200.0, | ||
| "storage_limit_mb": 500, | ||
| "private_minutes": 1200.0, | ||
| "public_minutes": 300.0, | ||
| "public_storage_avg_mb": 50.0, | ||
| }, | ||
| "copilot": None, | ||
| } | ||
| lines = _format_forecast_section( | ||
| data, include_forecast=True, reference_date=date(2026, 8, 5) | ||
| ) | ||
| joined = "\n".join(lines) | ||
| self.assertIn("private repos", joined) | ||
| self.assertIn("Public repos (free)", joined) | ||
| self.assertIn("300.0 min", joined) | ||
|
|
||
| def test_format_html_forecast_section_shows_private_scope_when_split_available(self): | ||
| from datetime import date | ||
|
|
||
| from github_usage.email_report_html import _format_html_forecast_section | ||
|
|
||
| data = { | ||
| "actions": { | ||
| "minutes": 1500.0, | ||
| "minutes_limit": 2000, | ||
| "storage_avg_mb": 200.0, | ||
| "storage_limit_mb": 500, | ||
| "private_minutes": 1200.0, | ||
| "public_minutes": 300.0, | ||
| "public_storage_avg_mb": 50.0, | ||
| }, | ||
| "copilot": None, | ||
| } | ||
| parts = _format_html_forecast_section( | ||
| data, include_forecast=True, reference_date=date(2026, 8, 5) | ||
| ) | ||
| html_body = "\n".join(parts) | ||
| self.assertIn("private repos", html_body) | ||
| self.assertIn("visibility-tag", html_body) | ||
| self.assertIn("300.0 min", html_body) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Split the report-formatting tests into a focused module.
tests/test_email_report.py now exceeds 960 lines. Move these public/private usage and forecast tests into a dedicated test module, such as tests/test_email_report_visibility.py.
As per coding guidelines, keep Python files below approximately 500 lines, begin extracting submodules or helpers near 400 lines, and favor small focused modules.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_email_report.py` around lines 835 - 961, Extract the
public/private usage, public-repository note, and forecast visibility tests from
test_email_report.py into a focused module such as
test_email_report_visibility.py. Preserve all test names, imports, fixtures,
assertions, and behavior while keeping the original module focused and below the
project’s preferred file size.
Source: Coding guidelines
…, extract visibility tests gui/wizard/__init__.py: re-raises ModuleNotFoundError when exc.name is not None and doesn't start with "textual", so only the optional Textual dependency is silently suppressed; broken internal imports surface immediately. tests/test_email_report_visibility.py: new focused module holding the 9 public/private split tests (actions split, footnote helpers, forecast scope) extracted from test_email_report.py, bringing that file back under 835 lines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- gui/wizard/__init__.py: assign __all__ = [] in the except branch so star-imports and introspection tools don't raise AttributeError when Textual is not installed (HIGH #1) - email_report_html.py: remove two no-op html.escape() calls — one on the already-safe storage_part format string, one on _run_out() which only ever returns "day {int}" or "--" (MEDIUM #3/#4) - email_report_html.py: remove redundant outer parentheses on the day-of-month paragraph in the parts list (LOW #5) - test_email_report_visibility.py: add missing edge-case test for pub_min > 0 with pub_mb == 0 in HTML formatter (LOW #6) - CHANGELOG.md: document HTML workflow default change with migration guidance for users who re-run setup (HIGH #2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Email Format Support
Wizard UI
Report Output
Tests
Summary by CodeRabbit
New Features
Bug Fixes
Documentation